Add c10::irange to ExecuTorch #8554
Conversation
irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/8554
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 2 PendingAs of commit 1cd3b5c with merge base cc3974f ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D69817195 |
irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/) ghstack-source-id: 267074908 Pull Request resolved: #8554
| @@ -26,6 +26,7 @@ def define_common_targets(): | |||
| "util/TypeSafeSignMath.h", | |||
| "util/bit_cast.h", | |||
| "util/floating_point_utils.h", | |||
| "util/irange.h", | |||
There was a problem hiding this comment.
is irange.h not part of headers from pytorch? or we decided that is not worth sharing?
There was a problem hiding this comment.
it is a pytorch header, but it is used in executorch core. therefore it must be copied over just like everything else in runtime/core/portable_type/c10/c10 to keep the build hermetic for embedded users
| @@ -0,0 +1,123 @@ | |||
| // Copyright 2004-present Facebook. All Rights Reserved. | |||
There was a problem hiding this comment.
Fix the copy right in line with rest of the codebase?
| @@ -0,0 +1,123 @@ | |||
| // Copyright 2004-present Facebook. All Rights Reserved. | |||
|
|
|||
| #pragma once | |||
There was a problem hiding this comment.
If this file is copied over, can you add a comment here to the effect?
There was a problem hiding this comment.
can you add a comment
no. the file should be identical to the version in pytorch core.
|
|
||
| #include <c10/util/TypeSafeSignMath.h> | ||
|
|
||
| #include <algorithm> |
There was a problem hiding this comment.
does it need <algorithm>?
There was a problem hiding this comment.
not relevant; this is a direct copy from pytorch core.
| @@ -11,6 +11,8 @@ | |||
| #include <algorithm> | |||
There was a problem hiding this comment.
not related to this diff per se, but makes me curious why we need in this file too.
| @@ -0,0 +1,123 @@ | |||
| // Copyright 2004-present Facebook. All Rights Reserved. | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
No. this is coming directly from pytorch core. https://github.com/pytorch/pytorch/blob/main/c10/util/irange.h
irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/) [ghstack-poisoned]
|
This pull request was exported from Phabricator. Differential Revision: D69817195 |
Pull Request resolved: #8554 irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. ghstack-source-id: 267193301 @exported-using-ghexport Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/)
irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/) [ghstack-poisoned]
|
This pull request was exported from Phabricator. Differential Revision: D69817195 |
Pull Request resolved: #8554 irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. ghstack-source-id: 267194238 @exported-using-ghexport Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/)
swolchok
added
the
release notes: api
|
checked lint; it's an unrelated cadence issue. merging |
swolchok
temporarily deployed
to
cherry-pick-bot
GitHub Actions
Inactive
Pull Request resolved: #8554 irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. ghstack-source-id: 267194238 @exported-using-ghexport Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/) Co-authored-by: Github Executorch <[email protected]>
Stack from ghstack (oldest at bottom):
irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop.
Differential Revision: D69817195